-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to dnd-kit for drag and drop #2239
Conversation
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset (largest 100 files by percent change)
View detailed bundle breakdownAdded
Removed
Bigger
Smaller
Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller
Unchanged No assets were unchanged |
ooh, thats nice. Are you hoping to use this to rearrange transactions too? |
Yeah, also hoping this fixes drag and drop in mobile as well. |
Firefox 121.0.1 (64-bit) (on desktop) Easily reproduced error doing dragging and drop of categories although I'm not sure on the exact steps as there seems to be some volatility. Show error from actual:
Same idea but crashing as soon as I moved a category after reloading:
|
Thank you @Teprifer for catching that! The screenshots were also very helpful. I have pushed a commit which hopefully fixes the issue :) |
something is weird with the sidebar account sorting. If I try to move an account by one place it usually doesn't move to the right location. Moving up seems to work better than moving down. Moving categories seems to work perfect |
@youngcw Accounts sorting should be working fine now :) |
@joel-jeremy Im still seeing the same issue.
|
The same problem seem to exist in the latest release version of Actual Budget and only happens with a fresh test budget. We might need to look at how the test file inserts the sort order of the accounts, but we should probably fix that in a separate PR. |
I do see it if I import one of my budget files, but it is less broken. I can move the accounts around a bit before they decide to jump to the bottom. Since this issue is existing, I think its fine to ignore in this PR. |
I can replicate this on the demo file in the deploy PR
|
Upon further investigation, it looks like importing / creating test budget incorrectly sets the |
@Teprifer I have added a bit of delay before expanding back the category groups. Animation should be smoother now. I also have updated the accounts in the mobile accounts page to be sortable. |
Created #2264 for the account sorting bug. |
It wasn't what I meant, but that added delay feels really good, creates a nice continuity feeling from drop -> see placement -> UI restores to expanded view, so keep it. :) I was referring to when you let go of a category or account, the one you're holding jumps up and down a bit, before it settles in to place. This weird hopping animation doesn't happen with groups. Seems to be it tries to go where it originally was, then immediately goes to where you'd placed it. Minor visual issue on mobile accounts, the account that is moved retains the selected/mouse-over highlight after being moved. This only occurred on an actual mobile device using a finger to tap & hold, when resizing on desktop to the mobile size this doesn't happen because the mouse cursor is over the one that was moved. |
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This PR was closed because it has been stalled for 5 days with no activity. |
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
This PR was closed because it has been stalled for 5 days with no activity. |
Main goal of the switch is to have a usable drag-and-drop functionality in mobile.